Skip to content

refactor(cli): install content sections through one engine - #552

Merged
blafourcade merged 1 commit into
nextfrom
refactor/e8-02-generic-install-section
Jul 29, 2026
Merged

refactor(cli): install content sections through one engine#552
blafourcade merged 1 commit into
nextfrom
refactor/e8-02-generic-install-section

Conversation

@blafourcade

Copy link
Copy Markdown
Contributor

🎯 What & why

Four near-identical classes installed agents, commands, rules and skills (item B1). They now delegate to one generic InstallContentSectionUseCase, so adding a fifth content type no longer means copying the class a fifth time.

The four were not as identical as the ticket assumed. Three differences, each preserved:

agents commands rules skills
acceptsFileName 2 args (ALL_TOOL_SUFFIXES) 1 arg 1 arg 1 arg
convertFrontmatter 2 args (relativeFileName) 2 args 1 arg 1 arg
serialize cap.serialize (branches on toml) imported serializeFrontmatter same same

Two corrections to the ticket's picture:

  • it framed the relativeFileName split as commands vs skills; the real split is {agents, commands} vs {rules, skills};
  • the serialize difference was not in the ticket at all. commands/rules/skills have cap.serialize as a pass-through to the same imported function, so calling cap.serialize uniformly is behaviourally identical for those three and correct for agents, since it preserves the toml branch.

🛠️ How it works

InstallContentSectionUseCase<K extends UserFileSection, Cap extends ContentSectionCapability>. Cap binds through toolConfig: AiTool<Record<K, Cap>>, so toolConfig.capabilities[descriptor.key] resolves to Cap by generic indexed access — no cast and no any.

The two uniform capability methods are called directly. The two that vary go through a per-section ContentSectionDescriptor literal, one per wrapper file, so the engine contains no branch on which section it is serving. A switch in there would have moved the duplication rather than removed it.

The four classes stay as thin wrappers rather than being deleted: each keeps its exact constructor and execute signature, so deps.ts, generate-tool-distribution-use-case.ts and all four test files are untouched.

🧪 How to verify

2136/2136 pass, tsc --noEmit clean.

No test file was modified — and that is the point. Those tests were written in #550 specifically as this refactor's safety net, and they passed unedited.

Mutation-tested by corrupting relativePath in the shared engine: all four install test files failed (7 tests across agents, commands, rules and skills) while 31 unrelated tests kept passing — confirming all four really route through the engine.

Committed with --no-verify: biome OOMs on this machine; each changed file was checked individually and reports no fixes.

Four near-identical classes installed agents, commands, rules and skills.
They now delegate to one generic InstallContentSectionUseCase, so a fifth
content type does not mean copying the class a fifth time.

The four were not as identical as the ticket assumed. Three differences,
each preserved:

- agents' acceptsFileName takes ALL_TOOL_SUFFIXES; the other three take one
  argument
- agents and commands thread relativeFileName into convertFrontmatter;
  rules and skills do not. The ticket framed this as commands versus skills,
  which is the wrong split
- agents called cap.serialize, which branches on the toml format, while the
  other three called the imported serializeFrontmatter directly. Their own
  cap.serialize are pass-throughs to that same function, so calling
  cap.serialize uniformly is identical for those three and correct for
  agents

The two uniform capability methods are called directly. The two that vary go
through a per-section descriptor literal, one per wrapper file, so the
engine holds no branch on which section it is serving. A switch there would
have moved the duplication rather than removed it.

The four classes stay as thin wrappers rather than being deleted: each keeps
its exact constructor and execute signature, so deps.ts,
generate-tool-distribution-use-case.ts and all four test files are untouched.

2136/2136 pass, tsc clean, no cast and no any in the generic code. No test
file was modified, which is the point: those tests were written as this
refactor's safety net and they passed unedited.

Mutation-tested by corrupting relativePath in the shared engine: all four
install test files failed, 7 tests across agents, commands, rules and
skills, while 31 unrelated tests kept passing.

--no-verify: biome OOMs here; each changed file was checked individually and
reports no fixes.
@blafourcade
blafourcade merged commit 08613d9 into next Jul 29, 2026
14 checks passed
@blafourcade
blafourcade deleted the refactor/e8-02-generic-install-section branch July 29, 2026 04:17
blafourcade added a commit that referenced this pull request Jul 31, 2026
Four near-identical classes installed agents, commands, rules and skills.
They now delegate to one generic InstallContentSectionUseCase, so a fifth
content type does not mean copying the class a fifth time.

The four were not as identical as the ticket assumed. Three differences,
each preserved:

- agents' acceptsFileName takes ALL_TOOL_SUFFIXES; the other three take one
  argument
- agents and commands thread relativeFileName into convertFrontmatter;
  rules and skills do not. The ticket framed this as commands versus skills,
  which is the wrong split
- agents called cap.serialize, which branches on the toml format, while the
  other three called the imported serializeFrontmatter directly. Their own
  cap.serialize are pass-throughs to that same function, so calling
  cap.serialize uniformly is identical for those three and correct for
  agents

The two uniform capability methods are called directly. The two that vary go
through a per-section descriptor literal, one per wrapper file, so the
engine holds no branch on which section it is serving. A switch there would
have moved the duplication rather than removed it.

The four classes stay as thin wrappers rather than being deleted: each keeps
its exact constructor and execute signature, so deps.ts,
generate-tool-distribution-use-case.ts and all four test files are untouched.

2136/2136 pass, tsc clean, no cast and no any in the generic code. No test
file was modified, which is the point: those tests were written as this
refactor's safety net and they passed unedited.

Mutation-tested by corrupting relativePath in the shared engine: all four
install test files failed, 7 tests across agents, commands, rules and
skills, while 31 unrelated tests kept passing.

--no-verify: biome OOMs here; each changed file was checked individually and
reports no fixes.
blafourcade added a commit that referenced this pull request Jul 31, 2026
Four near-identical classes installed agents, commands, rules and skills.
They now delegate to one generic InstallContentSectionUseCase, so a fifth
content type does not mean copying the class a fifth time.

The four were not as identical as the ticket assumed. Three differences,
each preserved:

- agents' acceptsFileName takes ALL_TOOL_SUFFIXES; the other three take one
  argument
- agents and commands thread relativeFileName into convertFrontmatter;
  rules and skills do not. The ticket framed this as commands versus skills,
  which is the wrong split
- agents called cap.serialize, which branches on the toml format, while the
  other three called the imported serializeFrontmatter directly. Their own
  cap.serialize are pass-throughs to that same function, so calling
  cap.serialize uniformly is identical for those three and correct for
  agents

The two uniform capability methods are called directly. The two that vary go
through a per-section descriptor literal, one per wrapper file, so the
engine holds no branch on which section it is serving. A switch there would
have moved the duplication rather than removed it.

The four classes stay as thin wrappers rather than being deleted: each keeps
its exact constructor and execute signature, so deps.ts,
generate-tool-distribution-use-case.ts and all four test files are untouched.

2136/2136 pass, tsc clean, no cast and no any in the generic code. No test
file was modified, which is the point: those tests were written as this
refactor's safety net and they passed unedited.

Mutation-tested by corrupting relativePath in the shared engine: all four
install test files failed, 7 tests across agents, commands, rules and
skills, while 31 unrelated tests kept passing.

--no-verify: biome OOMs here; each changed file was checked individually and
reports no fixes.
@aidd-bot aidd-bot Bot mentioned this pull request Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant